Skip to content

feat(avm): constrained ec_add#11525

Merged
IlyasRidhuan merged 1 commit intomasterfrom
ir/01-10-feat_avm_constrained_ec_add
Feb 13, 2025
Merged

feat(avm): constrained ec_add#11525
IlyasRidhuan merged 1 commit intomasterfrom
ir/01-10-feat_avm_constrained_ec_add

Conversation

@IlyasRidhuan
Copy link
Contributor

@IlyasRidhuan IlyasRidhuan commented Jan 27, 2025

Please read contributing guidelines and remove this line.

@IlyasRidhuan IlyasRidhuan mentioned this pull request Jan 27, 2025
Merged
@IlyasRidhuan Graphite App
Copy link
Contributor Author

IlyasRidhuan commented Jan 27, 2025 •
edited
Loading

This stack of pull requests is managed by Graphite. Learn more about stacking.

@IlyasRidhuan IlyasRidhuan mentioned this pull request Jan 27, 2025
Merged
@IlyasRidhuan IlyasRidhuan force-pushed the ir/01-10-feat_avm_constrained_ec_add branch 3 times, most recently from cfe6eb8 to 96cf39e Compare January 27, 2025 12:14
@IlyasRidhuan IlyasRidhuan marked this pull request as ready for review January 27, 2025 12:14
@IlyasRidhuan IlyasRidhuan removed the request for review from Maddiaa0 January 27, 2025 12:14
@IlyasRidhuan IlyasRidhuan force-pushed the ir/01-03-feat_constrain_sha256 branch from 1d2145b to 87a6c07 Compare January 27, 2025 12:24
@IlyasRidhuan IlyasRidhuan force-pushed the ir/01-10-feat_avm_constrained_ec_add branch from 96cf39e to 7d68185 Compare January 27, 2025 12:24
fcarreiro
fcarreiro reviewed Jan 27, 2025
View reviewed changes
barretenberg/cpp/src/barretenberg/vm2/generated/flavor.cpp Outdated
=======
AvmFlavor::CommitmentLabels::CommitmentLabels()
{
Base::precomputed_bitwise_input_a = "PRECOMPUTED_BITWISE_INPUT_A";
Copy link
Contributor

@fcarreiro fcarreiro Jan 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just update bbpilcom and regenerate all ;)

@fcarreiro fcarreiro self-requested a review January 27, 2025 12:26
fcarreiro
fcarreiro requested changes Jan 27, 2025
View reviewed changes
fcarreiro
fcarreiro requested changes Jan 27, 2025
View reviewed changes
barretenberg/cpp/src/barretenberg/vm2/tracegen/ecc_trace.cpp Outdated

namespace {

FF compute_lambda(const bool double_predicate, const bool add_predicate, GrumpkinPoint p, GrumpkinPoint q)
Copy link
Contributor

@fcarreiro fcarreiro Jan 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GrumpkinPoint is probably an expensive struct, pass as reference (ideally const&). Same everywhere else.

@IlyasRidhuan IlyasRidhuan force-pushed the ir/01-03-feat_constrain_sha256 branch from 87a6c07 to 8be1d44 Compare January 27, 2025 15:27
@IlyasRidhuan IlyasRidhuan force-pushed the ir/01-10-feat_avm_constrained_ec_add branch from 7d68185 to 5c36eec Compare January 27, 2025 16:03
Base automatically changed from ir/01-03-feat_constrain_sha256 to master January 28, 2025 10:25
@fcarreiro fcarreiro self-requested a review January 28, 2025 14:59
@IlyasRidhuan IlyasRidhuan force-pushed the ir/01-10-feat_avm_constrained_ec_add branch from 5c36eec to 5cc7534 Compare January 28, 2025 15:44
fcarreiro
fcarreiro approved these changes Jan 28, 2025
View reviewed changes
@IlyasRidhuan IlyasRidhuan mentioned this pull request Jan 29, 2025
Merged
jeanmon
jeanmon reviewed Jan 30, 2025
View reviewed changes
jeanmon
jeanmon reviewed Jan 30, 2025
View reviewed changes
jeanmon
jeanmon reviewed Jan 30, 2025
View reviewed changes
jeanmon
jeanmon reviewed Jan 30, 2025
View reviewed changes
barretenberg/cpp/pil/vm2/ecc.pil Outdated
// (1) Either p or q is infinity but not both (covered by result_infinity), the result is the non-infinity point
// (2) The result is infinity, result is (INFINITY_X, INFINITY_Y)
// (3) Neither point is infinity and NOT RESULT_INFINITY, result is (COMPUTED_R_X, COMPUTED_R_Y)
pol EITHER_INF = (1 - BOTH_INF) * (p_is_inf + q_is_inf);
Copy link
Contributor

@jeanmon jeanmon Jan 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would reformulate EITHER_INF as with the inlining of BOTH_INF, we get a degree 3 relation. What you want is a XOR of p_is_inf and q_is_inf which can be obtained by a relation of degree 2 as follows:
pol EITHER_INF = p_is_inf + q_is_inf - 2 * BOTH_INF = p_is_inf + q_is_inf - 2 * p_is_inf * q_is_inf;

jeanmon
jeanmon reviewed Jan 30, 2025
View reviewed changes
jeanmon
jeanmon reviewed Jan 30, 2025
View reviewed changes
jeanmon
jeanmon reviewed Jan 30, 2025
View reviewed changes
jeanmon
jeanmon reviewed Jan 30, 2025
View reviewed changes
jeanmon
jeanmon reviewed Jan 30, 2025
View reviewed changes
jeanmon
jeanmon reviewed Jan 30, 2025
View reviewed changes
barretenberg/cpp/src/barretenberg/vm2/constraining/relations/ecc.test.cpp Outdated
// R = P + (-P) = O; , where O is the point at infinity
GrumpkinPoint q(p.x, -p.y);
// Infinity coordinates
bb::grumpkin::fq r_x = grumpkin::fq("0x183227397098d014dc2822db40c0ac2e9419f4243cdcb848a1f0fac9f8000001");
Copy link
Contributor

@jeanmon jeanmon Jan 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn-t there any constant for this convention of x coordinate for the infty point?
Also, in pil this should be a constant.

Copy link
Contributor Author

@IlyasRidhuan IlyasRidhuan Jan 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should it be a constant in pil? It is just baked into the constraints directly.

Copy link
Contributor

@jeanmon jeanmon Feb 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought we could generate this constant from a unique one defined in .nr and then generated for PIl, CPP, TS ..
Also, do not we need such a constant it for ecadd opcode?

jeanmon
jeanmon previously requested changes Jan 30, 2025
View reviewed changes
Copy link
Contributor

@jeanmon jeanmon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please address my feedback especially the comments in pil.

@IlyasRidhuan IlyasRidhuan force-pushed the ir/01-10-feat_avm_constrained_ec_add branch 2 times, most recently from 5a378e0 to d916f01 Compare January 30, 2025 19:01
@IlyasRidhuan IlyasRidhuan force-pushed the ir/01-10-feat_avm_constrained_ec_add branch 5 times, most recently from 7883080 to 73f9803 Compare February 12, 2025 22:57
@IlyasRidhuan IlyasRidhuan requested a review from jeanmon February 13, 2025 08:51
@IlyasRidhuan IlyasRidhuan force-pushed the ir/01-10-feat_avm_constrained_ec_add branch from 73f9803 to dece8ee Compare February 13, 2025 09:46
@IlyasRidhuan IlyasRidhuan enabled auto-merge (squash) February 13, 2025 10:16
@IlyasRidhuan IlyasRidhuan merged commit f8fe602 into master Feb 13, 2025
52 checks passed
@IlyasRidhuan IlyasRidhuan deleted the ir/01-10-feat_avm_constrained_ec_add branch February 13, 2025 10:32
@AztecBot AztecBot mentioned this pull request Feb 13, 2025
Merged
sklppy88 pushed a commit that referenced this pull request Feb 13, 2025
@AztecBot
chore(master): Release 0.76.4 (#11971)
10f0891 
🤖 I have created a release *beep* *boop*
---


<details><summary>aztec-package: 0.76.4</summary>

##
[0.76.4](aztec-package-v0.76.3...aztec-package-v0.76.4)
(2025-02-13)


### Miscellaneous

* **aztec-package:** Synchronize aztec-packages versions
</details>

<details><summary>barretenberg.js: 0.76.4</summary>

##
[0.76.4](barretenberg.js-v0.76.3...barretenberg.js-v0.76.4)
(2025-02-13)


### Miscellaneous

* Unify webpack dev server versions
([#11965](#11965))
([921d2cd](921d2cd))
</details>

<details><summary>aztec-packages: 0.76.4</summary>

##
[0.76.4](aztec-packages-v0.76.3...aztec-packages-v0.76.4)
(2025-02-13)


### Features

* `FunctionDefinition::as_typed_expr`
(noir-lang/noir#7358)
([5efdd57](5efdd57))
* Aes decryption oracle
([#11907](#11907))
([c4ce913](c4ce913))
* **avm:** Constrained ec_add
([#11525](#11525))
([f8fe602](f8fe602))
* **avm:** Interaction testing
([#11947](#11947))
([fc647eb](fc647eb))
* **avm:** Relation microbenchmarks
([#11974](#11974))
([95b581d](95b581d))
* **cli:** Add `--target-dir` option
(noir-lang/noir#7350)
([5efdd57](5efdd57))
* Indexed protocol contracts tree
([#11897](#11897))
([96e84d4](96e84d4))
* **performance:** Check sub operations against induction variables
(noir-lang/noir#7356)
([5efdd57](5efdd57))
* **performance:** Use unchecked ops based upon known induction
variables (noir-lang/noir#7344)
([5efdd57](5efdd57))
* Small blob fixes/improvements
([#11686](#11686))
([4eab9fc](4eab9fc))
* Update fee model
([#11953](#11953))
([2798d58](2798d58))
* Use brillig optimized sha256
([#11696](#11696))
([438c905](438c905))


### Bug Fixes

* Ci fixes
([#11973](#11973))
([6386f4e](6386f4e))
* **cli:** Only lock the packages selected in the workspace
(noir-lang/noir#7345)
([5efdd57](5efdd57))
* Deterministic generation of vkeys in ts
([#11951](#11951))
([7901cac](7901cac))
* Incorrect secondary file in LSP errors
(noir-lang/noir#7347)
([5efdd57](5efdd57))
* Lock git dependencies folder when resolving workspace
(noir-lang/noir#7327)
([5efdd57](5efdd57))
* Perform SSA constraints check on final SSA
(noir-lang/noir#7334)
([5efdd57](5efdd57))
* Remove deprecated artifacts
([#11979](#11979))
([4f0dce7](4f0dce7))
* Remove serial queue in broker facade
([#11956](#11956))
([3485b52](3485b52))
* **ssa:** Make the lookback feature opt-in
(noir-lang/noir#7190)
([5efdd57](5efdd57))


### Miscellaneous

* **avm:** Tracegen interactions assertion
([#11972](#11972))
([b865ccc](b865ccc))
* Avoid doing all brillig integer arithmetic on u128s
(noir-lang/noir#7357)
([5efdd57](5efdd57))
* Basic test for MSM in Noir to catch performance improvements and
regressions (noir-lang/noir#7341)
([5efdd57](5efdd57))
* Bump devnet boot node resources
([#11958](#11958))
([bbcdefc](bbcdefc))
* **ci:** Add Vecs and vecs to cspell
(noir-lang/noir#7342)
([5efdd57](5efdd57))
* Deprecate keccak256 (noir-lang/noir#7361)
([5efdd57](5efdd57))
* Fix warnings (noir-lang/noir#7330)
([5efdd57](5efdd57))
* Mark sha256 as deprecated from the stdlib
(noir-lang/noir#7351)
([5efdd57](5efdd57))
* Moving storage slot out of `NoteHeader`
([#11904](#11904))
([8c4bb1c](8c4bb1c))
* Normalize path displayed by `nargo new`
(noir-lang/noir#7328)
([5efdd57](5efdd57))
* Redo typo PR by osrm (noir-lang/noir#7238)
([5efdd57](5efdd57))
* Release Noir(1.0.0-beta.2)
(noir-lang/noir#6914)
([5efdd57](5efdd57))
* Remove foreign calls array from Brillig VM constructor
(noir-lang/noir#7337)
([5efdd57](5efdd57))
* Remove misleading output from `nargo check`
(noir-lang/noir#7329)
([5efdd57](5efdd57))
* Remove some unused types and functions in the AST
(noir-lang/noir#7339)
([5efdd57](5efdd57))
* Remove unnecessary constants
(noir-lang/noir#7326)
([5efdd57](5efdd57))
* Revive browser test before killing it
([#11964](#11964))
([cb47cc0](cb47cc0))
* Split acirgen into multiple modules
(noir-lang/noir#7310)
([5efdd57](5efdd57))
* Unify webpack dev server versions
([#11965](#11965))
([921d2cd](921d2cd))
</details>

<details><summary>barretenberg: 0.76.4</summary>

##
[0.76.4](barretenberg-v0.76.3...barretenberg-v0.76.4)
(2025-02-13)


### Features

* Aes decryption oracle
([#11907](#11907))
([c4ce913](c4ce913))
* **avm:** Constrained ec_add
([#11525](#11525))
([f8fe602](f8fe602))
* **avm:** Interaction testing
([#11947](#11947))
([fc647eb](fc647eb))
* **avm:** Relation microbenchmarks
([#11974](#11974))
([95b581d](95b581d))


### Miscellaneous

* **avm:** Tracegen interactions assertion
([#11972](#11972))
([b865ccc](b865ccc))
* Unify webpack dev server versions
([#11965](#11965))
([921d2cd](921d2cd))
</details>

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
AztecBot added a commit to AztecProtocol/barretenberg that referenced this pull request Feb 14, 2025
@AztecBot
chore(master): Release 0.76.4 (#11971)
24a825c 
🤖 I have created a release *beep* *boop*
---


<details><summary>aztec-package: 0.76.4</summary>

##
[0.76.4](AztecProtocol/aztec-packages@aztec-package-v0.76.3...aztec-package-v0.76.4)
(2025-02-13)


### Miscellaneous

* **aztec-package:** Synchronize aztec-packages versions
</details>

<details><summary>barretenberg.js: 0.76.4</summary>

##
[0.76.4](AztecProtocol/aztec-packages@barretenberg.js-v0.76.3...barretenberg.js-v0.76.4)
(2025-02-13)


### Miscellaneous

* Unify webpack dev server versions
([#11965](AztecProtocol/aztec-packages#11965))
([921d2cd](AztecProtocol/aztec-packages@921d2cd))
</details>

<details><summary>aztec-packages: 0.76.4</summary>

##
[0.76.4](AztecProtocol/aztec-packages@aztec-packages-v0.76.3...aztec-packages-v0.76.4)
(2025-02-13)


### Features

* `FunctionDefinition::as_typed_expr`
(noir-lang/noir#7358)
([5efdd57](AztecProtocol/aztec-packages@5efdd57))
* Aes decryption oracle
([#11907](AztecProtocol/aztec-packages#11907))
([c4ce913](AztecProtocol/aztec-packages@c4ce913))
* **avm:** Constrained ec_add
([#11525](AztecProtocol/aztec-packages#11525))
([f8fe602](AztecProtocol/aztec-packages@f8fe602))
* **avm:** Interaction testing
([#11947](AztecProtocol/aztec-packages#11947))
([fc647eb](AztecProtocol/aztec-packages@fc647eb))
* **avm:** Relation microbenchmarks
([#11974](AztecProtocol/aztec-packages#11974))
([95b581d](AztecProtocol/aztec-packages@95b581d))
* **cli:** Add `--target-dir` option
(noir-lang/noir#7350)
([5efdd57](AztecProtocol/aztec-packages@5efdd57))
* Indexed protocol contracts tree
([#11897](AztecProtocol/aztec-packages#11897))
([96e84d4](AztecProtocol/aztec-packages@96e84d4))
* **performance:** Check sub operations against induction variables
(noir-lang/noir#7356)
([5efdd57](AztecProtocol/aztec-packages@5efdd57))
* **performance:** Use unchecked ops based upon known induction
variables (noir-lang/noir#7344)
([5efdd57](AztecProtocol/aztec-packages@5efdd57))
* Small blob fixes/improvements
([#11686](AztecProtocol/aztec-packages#11686))
([4eab9fc](AztecProtocol/aztec-packages@4eab9fc))
* Update fee model
([#11953](AztecProtocol/aztec-packages#11953))
([2798d58](AztecProtocol/aztec-packages@2798d58))
* Use brillig optimized sha256
([#11696](AztecProtocol/aztec-packages#11696))
([438c905](AztecProtocol/aztec-packages@438c905))


### Bug Fixes

* Ci fixes
([#11973](AztecProtocol/aztec-packages#11973))
([6386f4e](AztecProtocol/aztec-packages@6386f4e))
* **cli:** Only lock the packages selected in the workspace
(noir-lang/noir#7345)
([5efdd57](AztecProtocol/aztec-packages@5efdd57))
* Deterministic generation of vkeys in ts
([#11951](AztecProtocol/aztec-packages#11951))
([7901cac](AztecProtocol/aztec-packages@7901cac))
* Incorrect secondary file in LSP errors
(noir-lang/noir#7347)
([5efdd57](AztecProtocol/aztec-packages@5efdd57))
* Lock git dependencies folder when resolving workspace
(noir-lang/noir#7327)
([5efdd57](AztecProtocol/aztec-packages@5efdd57))
* Perform SSA constraints check on final SSA
(noir-lang/noir#7334)
([5efdd57](AztecProtocol/aztec-packages@5efdd57))
* Remove deprecated artifacts
([#11979](AztecProtocol/aztec-packages#11979))
([4f0dce7](AztecProtocol/aztec-packages@4f0dce7))
* Remove serial queue in broker facade
([#11956](AztecProtocol/aztec-packages#11956))
([3485b52](AztecProtocol/aztec-packages@3485b52))
* **ssa:** Make the lookback feature opt-in
(noir-lang/noir#7190)
([5efdd57](AztecProtocol/aztec-packages@5efdd57))


### Miscellaneous

* **avm:** Tracegen interactions assertion
([#11972](AztecProtocol/aztec-packages#11972))
([b865ccc](AztecProtocol/aztec-packages@b865ccc))
* Avoid doing all brillig integer arithmetic on u128s
(noir-lang/noir#7357)
([5efdd57](AztecProtocol/aztec-packages@5efdd57))
* Basic test for MSM in Noir to catch performance improvements and
regressions (noir-lang/noir#7341)
([5efdd57](AztecProtocol/aztec-packages@5efdd57))
* Bump devnet boot node resources
([#11958](AztecProtocol/aztec-packages#11958))
([bbcdefc](AztecProtocol/aztec-packages@bbcdefc))
* **ci:** Add Vecs and vecs to cspell
(noir-lang/noir#7342)
([5efdd57](AztecProtocol/aztec-packages@5efdd57))
* Deprecate keccak256 (noir-lang/noir#7361)
([5efdd57](AztecProtocol/aztec-packages@5efdd57))
* Fix warnings (noir-lang/noir#7330)
([5efdd57](AztecProtocol/aztec-packages@5efdd57))
* Mark sha256 as deprecated from the stdlib
(noir-lang/noir#7351)
([5efdd57](AztecProtocol/aztec-packages@5efdd57))
* Moving storage slot out of `NoteHeader`
([#11904](AztecProtocol/aztec-packages#11904))
([8c4bb1c](AztecProtocol/aztec-packages@8c4bb1c))
* Normalize path displayed by `nargo new`
(noir-lang/noir#7328)
([5efdd57](AztecProtocol/aztec-packages@5efdd57))
* Redo typo PR by osrm (noir-lang/noir#7238)
([5efdd57](AztecProtocol/aztec-packages@5efdd57))
* Release Noir(1.0.0-beta.2)
(noir-lang/noir#6914)
([5efdd57](AztecProtocol/aztec-packages@5efdd57))
* Remove foreign calls array from Brillig VM constructor
(noir-lang/noir#7337)
([5efdd57](AztecProtocol/aztec-packages@5efdd57))
* Remove misleading output from `nargo check`
(noir-lang/noir#7329)
([5efdd57](AztecProtocol/aztec-packages@5efdd57))
* Remove some unused types and functions in the AST
(noir-lang/noir#7339)
([5efdd57](AztecProtocol/aztec-packages@5efdd57))
* Remove unnecessary constants
(noir-lang/noir#7326)
([5efdd57](AztecProtocol/aztec-packages@5efdd57))
* Revive browser test before killing it
([#11964](AztecProtocol/aztec-packages#11964))
([cb47cc0](AztecProtocol/aztec-packages@cb47cc0))
* Split acirgen into multiple modules
(noir-lang/noir#7310)
([5efdd57](AztecProtocol/aztec-packages@5efdd57))
* Unify webpack dev server versions
([#11965](AztecProtocol/aztec-packages#11965))
([921d2cd](AztecProtocol/aztec-packages@921d2cd))
</details>

<details><summary>barretenberg: 0.76.4</summary>

##
[0.76.4](AztecProtocol/aztec-packages@barretenberg-v0.76.3...barretenberg-v0.76.4)
(2025-02-13)


### Features

* Aes decryption oracle
([#11907](AztecProtocol/aztec-packages#11907))
([c4ce913](AztecProtocol/aztec-packages@c4ce913))
* **avm:** Constrained ec_add
([#11525](AztecProtocol/aztec-packages#11525))
([f8fe602](AztecProtocol/aztec-packages@f8fe602))
* **avm:** Interaction testing
([#11947](AztecProtocol/aztec-packages#11947))
([fc647eb](AztecProtocol/aztec-packages@fc647eb))
* **avm:** Relation microbenchmarks
([#11974](AztecProtocol/aztec-packages#11974))
([95b581d](AztecProtocol/aztec-packages@95b581d))


### Miscellaneous

* **avm:** Tracegen interactions assertion
([#11972](AztecProtocol/aztec-packages#11972))
([b865ccc](AztecProtocol/aztec-packages@b865ccc))
* Unify webpack dev server versions
([#11965](AztecProtocol/aztec-packages#11965))
([921d2cd](AztecProtocol/aztec-packages@921d2cd))
</details>

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fcarreiro fcarreiro fcarreiro approved these changes

@jeanmon jeanmon Awaiting requested review from jeanmon

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@IlyasRidhuan @fcarreiro @jeanmon